Skip to content

WIP: Python rewrite#58

Draft
asmacdo wants to merge 55 commits intocon:mainfrom
asmacdo:redesign-python
Draft

WIP: Python rewrite#58
asmacdo wants to merge 55 commits intocon:mainfrom
asmacdo:redesign-python

Conversation

@asmacdo
Copy link
Copy Markdown
Member

@asmacdo asmacdo commented Mar 30, 2026

Try it

pip install "con-yolo @ git+https://github.com/asmacdo/yolo@redesign-python"
yo demo

Why rewrite?

Every new tool was a PR to the Dockerfile and a debate about what belongs in the base image (#28, #31, #43, #39). Config was sourced bash, hard to layer, hard to test.
The architecture had no extension points: growth required modifying the core.

Now:

  • Config is YAML that merges.
  • Users can customize images, and keep them separate
  • Extras are composable scripts, not Dockerfile edits. packages: [numpy] in your config, or drop a bash script in .yolo/image-extras/ for anything complex. apt, datalad, git-delta, jj, pip, playwright, python are already optional extras. No Dockerfile knowledge needed.
  • Install is pip install, not git clone + manual setup. Editable install for quicker dev cycles.
  • Install separated from image build
  • Python with tests: unit tests, integration tests, pre-commit (ruff, shellcheck, yamllint), CI.

Legacy bash preserved in design/legacy/.

Closes:

Breaking changes

  • Config is YAML, not sourced bash. Migration: point Claude at your old
    config and SPEC.md, it generates the YAML.
  • CLI is yo build, yo run, etc. (not a single yolo command)
  • Image tags are yolo-<project>-<name>
New features
  • yo demo — interactive tour inside a container
  • yo clip — clipboard bridge (container → host)
  • yo images — list images and build status
  • Context injectioncontext: config key → --append-system-prompt
  • Auto-buildyo run builds if image doesn't exist
  • Named images with from: — podman-native layering from config
  • Composable image extras: bash scripts, not Dockerfiles
Config

Declarative YAML, merged from 5 locations (package defaults → system →
user → project → local). Lists append, dicts recurse, scalars replace.
!replace tag for overrides. See SPEC.md for full reference.

Testing
  • Unit tests for config, builder, launcher, CLI
  • Integration tests for every extra (verify mode + idempotency)
  • Pre-commit: ruff, ruff-format, yamllint, shellcheck

TODO

Resolve each before merge or file as a new issue.

  • Update install instructions for primary repo/branch
  • Decide: alias yoloyo run or change entry to yolo
  • Remove design/legacy/ (separate PR after cutover)
  • Consider adding more e2e integration tests that dont need claude subscription
  • Security audit
  • Add yo config to see merged config
  • Config migration script (bash → YAML)
  • Docs for writing custom image-extras scripts
  • Add set -eu to playwright.sh

asmacdo and others added 30 commits March 28, 2026 12:06
- Move bin/, images/, tests/, setup-yolo.sh, config.example, README.md to legacy/
- Add design/ with HACK_DECISIONS.md, REDESIGN_HACKIN.md, LEGACY_SPEC.md
- Add notes/ with issue and PR summaries from GitHub
- Organize test scripts into legacy/

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Legacy Dockerfile minus all --extras flags (cuda, playwright, datalad,
jj, git-annex, extra packages). Those become container-extras scripts
in the new architecture.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- FROM debian:bookworm instead of node:22
- Native Claude Code installer (no npm/Node.js dependency)
- Minimal packages: ca-certificates, curl, git, jq, sudo, tini, tree, vim, wget
- Non-root 'yolo' user with passwordless sudo
- Auto-update disabled (container is ephemeral)
- No language runtimes — those come from container-extras

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Containerfile.extras: static Dockerfile that layers container-extras
  on top of yolo-base via a build context with scripts + run.sh manifest
- apt.sh: simple wrapper around apt-get install
- python.sh: installs Python via uv with version arg, creates symlinks
- test-extras-build.sh: build + idempotency verification
- features_to_test.md: running checklist of things to test

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- pyproject.toml with click + pyyaml deps, pytest for dev
- config.py: load and merge YAML from 4 locations with list
  appending, dict recursion, scalar replacement
- cli.py: click entry point with stub build/run commands
- Entry point 'yo' (temporary, becomes 'yolo' at cutover)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
17 tests covering:
- _merge: scalar override, list append, dict recursion, type mismatch
- _config_paths: /etc, XDG default/override, project, precedence order
- load_config: empty, single file, multi-layer merge, git/yolo config,
  git overrides project

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Replace pyyaml with ruamel.yaml for round-trip config support
- Add builder.py: resolves container-extras from search path,
  assembles build context, invokes podman build
- Wire yo build command to builder
- Update tests for ruamel.yaml

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
17 tests covering:
- _parse_extras: bare names, prefixed, dict with args, mixed, empty
- _resolve_script: found, missing, later-path-wins
- _collect_apt_fallbacks: batching, script preservation, prefix passthrough
- assemble_build_context: run.sh generation, script copying, missing script
  error, args in manifest

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Decision con#5: all container-extras use name: + params form. Params
become YOLO_{NAME}_{KEY} env vars passed to scripts.

- Rewrite _parse_extra for single format (remove prefix/simple dict)
- Replace $@ with env vars in apt.sh, python.sh, git-delta.sh
- Scripts validate their own required env vars
- Add default config with apt packages, git-delta, python 3.12
- Config loader loads package defaults before user/project configs
- Fix hyphen-to-underscore in env var names
- Rewrite builder tests for new contract

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Move legacy/ and ideas/ under design/
- Move issues, PRs, notes, security-reports, ideas to .local-notes/
- Break up TODO.md into individual files in .local-notes/TODO/
- Update CLAUDE.md paths

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- pre-commit config with ruff lint+format, yamllint, shellcheck
- New CI workflow with lint and test jobs (replaces legacy CI)
- Move legacy ci.yml to design/legacy/
- CONTRIBUTING.md with setup, testing, and container-extras guide
- Fix unused imports, shellcheck warnings, YAML document starts
- yamllint config to allow bare 'on' key in GitHub Actions

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Separate unit-tests and integration-tests jobs
- Integration depends on lint + unit passing
- Commented-out matrix for python 3.11/3.12/3.13 x linux/mac/windows
- codespell and shellcheck as standalone jobs
- Integration stubs fail with TODO messages

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Hardcoded v1 — mounts claude config, gitconfig, workspace,
sets up env vars, container naming, runs claude with
--dangerously-skip-permissions. No config integration yet.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- click UNPROCESSED args forwarded to claude command
- yo run --help shows [CLAUDE_ARGS]...

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Launcher reads volumes from config and CLI -v flags
- Config volumes and CLI volumes both added as bind mounts
- 8 new launcher tests (command assembly, args, volumes)
- Updated CLAUDE.md: imports at top convention

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Custom entrypoint skips --dangerously-skip-permissions (legacy behavior).
TODO: make skip_permissions a separate config value.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Config key 'container-extras' becomes 'images' list, each with
  name + extras
- Image tags derived from project dirname: yolo-<project>-<name>
- 'from' key overrides BASE_IMAGE build arg (podman handles composition)
- Containerfile.extras uses ARG BASE_IMAGE=yolo-base
- CLI: yo build --image, yo run --image
- Remove TODO about renaming volumes

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replaces the brainstorming hackpad with an accurate spec covering:
config format/locations/merging, named images with FROM support,
container-extras contract (env vars), script resolution, launcher
behavior, and security posture.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Check if base image exists before building extras
- yolo-base: auto-build from Containerfile.base
- Config-defined image: build that first
- Unknown image: error with clear message

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- _detect_worktree: parse .git file/symlink, find original repo
- _worktree_volume: handle modes (ask prompts, bind mounts, skip
  ignores, error exits)
- Config key 'worktree' with CLI --worktree override
- 7 new tests for detection and volume behavior

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
~/projects → 1-to-1 mount with :z
~/data::ro → 1-to-1 with custom options
/host:/container → partial, :z appended
/host:/cont:opts → full form, unchanged

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Repeatable flag passes args directly to podman run.
Explicit opt-in, not blanket passthrough.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- --nvidia: CDI GPU passthrough with CDI spec warning
- --container-arg: repeatable raw arg to container engine
- nvidia also configurable via config key
- Renamed from --podman-arg to --container-arg for runtime agnosticism

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Values tagged !replace in YAML replace instead of appending:

  extras: !replace
    - datalad

Allows overriding default lists without repeating everything.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Two failing test cases documenting the desired behavior:
- Deep named-list merge (images/extras merged by name)
- Deep !replace inside named lists
TODO: implement recursive merge for lists of named dicts

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
env key in config: bare names pass through from host,
KEY=VALUE sets explicitly. Defaults pass through
CLAUDE_CODE_OAUTH_TOKEN and CLAUDE_CODE_EXPERIMENTAL_AGENT_TEAMS.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
asmacdo and others added 23 commits March 29, 2026 18:24
Explains config layering, merge rules, !replace, volume shorthand,
extras script resolution path, and env var contract.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- tests/integration/ for slow tests needing podman
- pytest marker 'integration' auto-applied via conftest
- Default: pytest -m 'not integration' skips them
- Move test-extras-build.sh to integration/

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Scripts support YOLO_VERIFY=1: set test defaults, install, verify.
Integration tests build through Containerfile.extras (real path),
test both verify and idempotency for all scripts.

- apt.sh: installs figlet, verifies command exists
- python.sh: installs 3.12, verifies python/python3 symlinks
- git-delta.sh: installs 0.18.2, verifies delta command

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Print image name, base, and extras with source paths before building
- run.sh uses PS4='+ [yolo] ' and set -eux for clear trace output
- Echo ==> script_name before each extras script

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- config.yaml: real defaults loaded by code (no comments)
- config.template.yaml: all commented, copied by yo init
- Template is documentation, defaults are functional

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Gitignore .yolo/ — it's per-project config, not repo content.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Build with YOLO_VERIFY=1 injected into run.sh — scripts install
AND verify in the same build pass. No separate command needed.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Checks `podman image exists` before launching. If the image is missing,
runs build() automatically instead of falling through to podman's
registry search.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Composes `context` list from config and passes it to Claude as a single
--append-system-prompt arg. Config layers append as usual, so defaults,
user, and project context all combine.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Container Claude writes to /tmp/yolo-clip/content, user runs `yo clip`
on the host to pipe it to their clipboard. Configurable via
`host_clipboard_command` (default: xclip -selection clipboard).
Default context line tells Claude about the mechanism.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
`yo demo` chdirs to the bundled demo/ directory and launches a container
with the demo script as the initial prompt. Removes the need for a
separate yo-demo project directory.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
`yo demo` copies the bundled demo script to a tmpdir and launches a
container there. Demo files live in src/yolo/demo/ so they ship with
the package via importlib.resources.

Early clipboard step writes the tmpdir path so the presenter can cd
there on the host.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Ports the remaining extras from the legacy bash implementation:
- datalad: installs via uv tool with datalad-container and datalad-next
- jj: downloads musl binary from GitHub releases, adds zsh completions
- playwright: installs nodejs/npm, system deps, and chromium

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Shows each image name, its podman tag, and whether it's been built.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Demo asks about user's experience and tailors accordingly
- Offers feature topics as user scenarios instead of scripted walkthrough
- Mounts yolo source read-only at /opt/yolo for showing real code
- Context config tells Claude to quote tool output and abort gracefully
- Removes old demo/demo.md (now in src/yolo/demo/)
- SPEC.md: mark context injection done, add claude-to-claude TODO

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- shellcheck: fix scandir from container-extras to image-extras
- unit-tests: run all non-integration tests instead of listing files
- integration-tests: build base image and run pytest -m integration

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Installs packages globally via uv tool so they persist across
sessions without needing a project venv.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@yarikoptic
Copy link
Copy Markdown
Member

FWIW I would need config migration to adopt... Indeed not sure how/who would review etc

@yarikoptic
Copy link
Copy Markdown
Member

FWIW I just used mounts config so far, so could be easy to migrate by simply sourcing and printing to populate values in yaml

@asmacdo
Copy link
Copy Markdown
Member Author

asmacdo commented Mar 30, 2026

@yarikoptic config is similar, format is different. claude (not yolo, so it can see all the paths) should be able to migrate

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Python rewrite of the yolo toolchain to run Claude Code inside a rootless Podman container, shifting from a legacy bash implementation to a packaged, tested Python CLI (yo) with YAML config layering and composable “image extras”.

Changes:

  • Added Python modules for config loading/merging, image building, and container launching (src/yolo/*) with a Click-based CLI (yo ...).
  • Introduced Containerfiles + composable image-extras/*.sh installers, plus unit/integration tests and CI updates.
  • Updated docs/specs (README/SPEC/CONTRIBUTING/CLAUDE) and preserved the legacy bash implementation under design/legacy/.

Reviewed changes

Copilot reviewed 44 out of 53 changed files in this pull request and generated 18 comments.

Show a summary per file
File Description
src/yolo/cli.py Implements yo CLI commands (build/run/init/images/clip/demo).
src/yolo/config.py YAML config discovery, loading, merge rules, and !replace tag support.
src/yolo/builder.py Resolves extras, assembles build contexts, and runs podman build.
src/yolo/launcher.py Assembles and executes podman run for Claude Code sessions.
src/yolo/defaults/config.yaml Ships default config (env passthrough, default image + extras).
src/yolo/defaults/config.template.yaml Template for yo init config generation.
src/yolo/demo/demo.md Demo script/instructions for interactive tour.
src/yolo/demo/.yolo/config.yaml Demo-specific context injection config.
src/yolo/__init__.py Package marker.
images/Containerfile.base Defines minimal Debian base image + Claude Code install.
images/Containerfile.extras Layers “extras” scripts on top of a base image.
image-extras/apt.sh Extra installer script for apt packages (+ verify mode).
image-extras/python.sh Extra installer script for Python via uv (+ verify mode).
image-extras/pip.sh Extra installer script for pip packages via uv tool (+ verify mode).
image-extras/git-delta.sh Extra installer script for git-delta (+ verify mode).
image-extras/datalad.sh Extra installer script for DataLad (+ verify mode).
image-extras/jj.sh Extra installer script for Jujutsu (+ verify mode).
image-extras/playwright.sh Extra installer script for Playwright/Chromium (+ verify mode).
tests/test_cli.py Unit tests for CLI behavior (clip).
tests/test_config.py Unit tests for config path precedence, merge rules, !replace.
tests/test_builder.py Unit tests for extras parsing, script resolution, build context, image tags.
tests/test_launcher.py Unit tests for launcher command assembly, volumes, context injection, auto-build.
tests/integration/conftest.py Marks integration tests.
tests/integration/test_image_extras.py Integration tests building/verifying each extra script via Podman.
tests/integration/test-extras-build.sh Shell integration test for extras build/idempotency.
tests/integration/__init__.py Integration test package marker.
tests/features_to_test.md Test checklist for remaining integration behaviors.
.github/workflows/ci.yml New CI workflow for lint/unit/integration tests.
.pre-commit-config.yaml Adds ruff/yamllint/shellcheck hooks.
.yamllint.yml Yamllint config.
pyproject.toml Defines Python package metadata and yo entrypoint.
.gitignore Ignores venv/build artifacts and .yolo/ (with demo exception).
.gitmodules Updates bats submodule paths (currently inconsistent with repo layout).
README.md Updated user-facing docs for Python rewrite and yo usage.
SPEC.md Updated spec for Python rewrite (config/images/extras/launcher).
CONTRIBUTING.md Contributor setup/test guidance for the Python rewrite.
CLAUDE.md Repo guidance and key-file index.
context-injection-notes.md Notes on context injection approach.
design/HACK_DECISIONS.md Locked design decisions for the rewrite.
design/LEGACY_SPEC.md Preserved spec of the legacy bash implementation.
design/legacy/* Preserved legacy implementation, tests, and CI config for reference.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +38 to +48
@main.command()
@click.option("--local", "target", flag_value="local", help="Write to .git/yolo/")
@click.option("--user", "target", flag_value="user", help="Write to ~/.config/yolo/")
@click.option("--path", "custom_path", default=None, help="Write to custom location")
@click.option(
"--project",
"target",
flag_value="project",
default=True,
help="Write to .yolo/ (default)",
)
Copy link

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The --project option uses flag_value="project" but sets default=True, which makes target a boolean by default rather than a string. It currently falls through to the project path by accident, but it's easy to break later and makes the type inconsistent. Prefer default="project" (or default=None and treat None as project) so target is always a string.

Copilot uses AI. Check for mistakes.
Comment on lines +172 to +177
f"--name={name}",
"-v",
f"{claude_dir}:{claude_dir}:z",
"-v",
f"{home}/.gitconfig:/tmp/.gitconfig:ro,z",
"-v",
Copy link

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yo run always bind-mounts ~/.gitconfig as a file, but doesn't ensure it exists. If the user has no ~/.gitconfig, podman will typically create the source path as a directory, and the container will see /tmp/.gitconfig as a directory (breaking git). Consider creating an empty ~/.gitconfig when missing (or making the mount conditional).

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should pull this out of the launcher, and put it into default config

Comment on lines +166 to +182
def _ensure_base(base: str, images_config: list) -> None:
"""Ensure a base image exists. Build it if we know how."""
if _image_exists(base):
return

if base == BASE_IMAGE:
_build_base()
return

# Check if it's an image defined in our config
for entry in images_config:
name = entry.get("name", "default")
if image_tag(name) == base:
build_image(entry, images_config)
return

raise RuntimeError(f"Base image '{base}' not found and not defined in config")
Copy link

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_ensure_base() only treats from: as either an existing podman image tag or the computed tag (image_tag(name)) of a configured image. But the spec/examples use from: base to refer to another configured image by name. As-is, from: base will raise at runtime unless the user writes the full tag. Consider allowing from: to match an images: entry by name (and translate to image_tag(name)), while still supporting arbitrary external image references.

Copilot uses AI. Check for mistakes.
Comment on lines +13 to +32
def _expand_volume(vol: str) -> str:
"""Expand volume shorthand to full podman -v syntax.

~/projects → $HOME/projects:$HOME/projects:z
~/data::ro → $HOME/data:$HOME/data:ro
/host:/container → /host:/container:z
/host:/cont:opts → /host:/cont:opts (unchanged)
"""
home = str(Path.home())
if "::" in vol:
path, _, opts = vol.partition("::")
path = path.replace("~", home, 1)
return f"{path}:{path}:{opts}"
elif vol.count(":") >= 2:
return vol
elif ":" in vol:
return f"{vol}:z"
else:
path = vol.replace("~", home, 1)
return f"{path}:{path}:z"
Copy link

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_expand_volume() doesn't expand ~ for the common host:container or host:container:opts forms. This is especially problematic for YAML config values like ~/data:/data, where the shell won't expand ~ and podman will receive it literally. Expand ~ on the host-path portion for : forms as well (or call Path(...).expanduser() before assembling the final string).

Copilot uses AI. Check for mistakes.
Comment on lines +8 to +10
BUILD_CONTEXT="$(mktemp -d "${TMPDIR:-/tmp}"/yolo-test-XXXXXXX)"
trap 'rm -rf $BUILD_CONTEXT' EXIT

Copy link

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This trap uses an unquoted $BUILD_CONTEXT in rm -rf, which will trigger ShellCheck (and can behave unexpectedly if the temp path contains whitespace). Quote the variable in the trap command (and consider using trap 'rm -rf -- "$BUILD_CONTEXT"' EXIT).

Copilot uses AI. Check for mistakes.
Comment on lines +9 to +14
REPO_ROOT = Path(__file__).resolve().parent.parent.parent
CONTAINERFILE_EXTRAS = REPO_ROOT / "images" / "Containerfile.extras"
BUILTIN_EXTRAS = REPO_ROOT / "image-extras"

BASE_IMAGE = "yolo-base"

Copy link

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

REPO_ROOT is computed relative to the installed module path, then used to locate images/ and image-extras/ outside the Python package. This will work in a source checkout but will break for wheel/PyPI installs because those directories are not part of src/yolo. Consider shipping Containerfiles + builtin extras as package resources (e.g., under src/yolo/resources/) and resolving them via importlib.resources, or adjust the build to include these directories in the wheel and update these paths accordingly.

Copilot uses AI. Check for mistakes.
Comment on lines +30 to +35
def image_tag(image_name: str) -> str:
"""Derive podman image tag from image name."""
project = _project_dirname()
project = "".join(c if c.isalnum() or c in "-_" else "-" for c in project)
return f"yolo-{project}-{image_name}"

Copy link

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image_tag() sanitizes the project dirname but not image_name. Config examples include names like myproject:heavy, and characters like : will produce invalid/unintended image references (and may be interpreted as a tag separator). Sanitize image_name the same way (and/or validate and raise a clear error for unsupported characters).

Copilot uses AI. Check for mistakes.
Comment on lines +21 to +26
[project.scripts]
yo = "yolo.cli:main"

[tool.hatch.build.targets.wheel]
packages = ["src/yolo"]

Copy link

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The wheel build only packages src/yolo, but the runtime expects top-level images/ and image-extras/ directories to exist (see yolo.builder paths). A pip install con-yolo wheel will therefore be missing required Containerfiles and builtin extras. Either move these assets under src/yolo/ and load via importlib.resources, or configure Hatch to include them in the wheel as package data and update the path resolution accordingly.

Copilot uses AI. Check for mistakes.
Comment on lines +156 to +161
tag = image_tag(image_name or "default")
result = subprocess.run(["podman", "image", "exists", tag], capture_output=True)
if result.returncode != 0:
print(f"Image {tag} not found, building...", file=sys.stderr)
build(config.get("images", []), only=image_name)

Copy link

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the image is missing, run() calls build(config.get("images", []), only=image_name) but doesn't validate that a matching image entry exists (or that any images are configured). If only doesn't match, no build happens and the subsequent podman run will fail with a less clear error. Detect this case and raise a user-facing error (e.g., ClickException) when the requested image name isn't defined or no images are configured.

Copilot uses AI. Check for mistakes.
Comment on lines +206 to +213
cmd += [
"claude",
"--dangerously-skip-permissions",
*context_args,
*(claude_args or []),
]

subprocess.run(cmd)
Copy link

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The final subprocess.run(cmd) result is ignored, so yo run can exit 0 even when podman run fails. Use check=True (and let Click propagate a non-zero exit), or explicitly sys.exit(result.returncode) so callers/CI can detect failures.

Copilot uses AI. Check for mistakes.
@yarikoptic
Copy link
Copy Markdown
Member

for such simple deterministic migrations there should IMHO be the code not reliance on agents to do the right thing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants